Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(1) Fixed error when using config habitat_hitl.episodes_filter='4' (2) Fixed episodes_filter episode order not respected (3) added check for data folder in current working directory #1774

Merged
merged 3 commits into from
Jan 26, 2024

Conversation

henrysamer
Copy link
Contributor

Motivation and Context

Fix error when using habitat_hitl.episodes_filter config value. Fixed episodes_filter episode order not respected Add check for required data/ directory at runtime. Update docs with data directory requirements.

Related to issues:
https://app.asana.com/0/0/1206342374687783/f
https://app.asana.com/0/0/1206329593074711/f
https://app.asana.com/0/0/1206363026162478/f

How Has This Been Tested

  • Locally tested habitat_hitl.episodes_filter config with string values containing integers (ex: '4') and verified no errors for minimal, basic_viewer, and pick_throw_vr apps.
  • Tested order of episodes when setting episodes_filter with edge cases.
  • Added data directory check in code and tested with missing data/ folder
  • Updated READMEs with example launch command directories

Types of changes

  • [Bug Fix] habitat_hitl.episodes_filter int casting issue
  • [Bug Fix] habitat_hitl.episodes_filter episode order not respected
  • [Docs change] Added data directory requirements to HITL documentation
  • [Bug Fix] Runtime check for data/ folder with error message

Checklist

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

…) Fixed episodes_filter episode order not respected (3) added check for data folder in current working directory
@henrysamer henrysamer added this to the Better engineering milestone Jan 26, 2024
@henrysamer henrysamer self-assigned this Jan 26, 2024
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jan 26, 2024
@henrysamer henrysamer marked this pull request as ready for review January 26, 2024 20:48
Copy link
Contributor

@eundersander eundersander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a stab at polishing/rewriting some of the documentation text. Otherwise LGTM!

habitat-hitl/README.md Outdated Show resolved Hide resolved
Updated README.md with reviewer rewording recommendation.
Modified Installation Step #5 in README.md
@henrysamer henrysamer merged commit 0ddebb1 into main Jan 26, 2024
1 of 3 checks passed
@henrysamer henrysamer deleted the hitl_fix2 branch January 26, 2024 22:21
dannymcy pushed a commit to dannymcy/habitat-lab that referenced this pull request Jul 8, 2024
…) Fixed episodes_filter episode order not respected (3) added check for data folder in current working directory (facebookresearch#1774)

* (1) Fixed error when using config habitat_hitl.episodes_filter='4' (2) Fixed episodes_filter episode order not respected (3) added check for data folder in current working directory

* Update README.md

Updated README.md with reviewer rewording recommendation.

* Update README.md

Modified Installation Step facebookresearch#5 in README.md
HHYHRHY pushed a commit to SgtVincent/habitat-lab that referenced this pull request Aug 31, 2024
…) Fixed episodes_filter episode order not respected (3) added check for data folder in current working directory (facebookresearch#1774)

* (1) Fixed error when using config habitat_hitl.episodes_filter='4' (2) Fixed episodes_filter episode order not respected (3) added check for data folder in current working directory

* Update README.md

Updated README.md with reviewer rewording recommendation.

* Update README.md

Modified Installation Step #5 in README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants